Skip to content

test: use error code mapping in place of raw errno#38675

Closed
RaisinTen wants to merge 1 commit intonodejs:masterfrom
RaisinTen:test/replace-raw-errno-with-error-code
Closed

test: use error code mapping in place of raw errno#38675
RaisinTen wants to merge 1 commit intonodejs:masterfrom
RaisinTen:test/replace-raw-errno-with-error-code

Conversation

@RaisinTen
Copy link
Member

@RaisinTen RaisinTen commented May 13, 2021

@RaisinTen RaisinTen added the blocked PRs that are blocked by other issues or PRs. label May 13, 2021
@github-actions github-actions bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels May 13, 2021
@waelsy123
Copy link
Contributor

waelsy123 commented May 22, 2021

any reason why this PR still blocked?

@RaisinTen
Copy link
Member Author

RaisinTen commented May 23, 2021

@waelsy123 This is still blocked because the linked PR hasn't landed in Node.js yet. We can unblock this after the next Libuv update PR lands here.

@RaisinTen RaisinTen marked this pull request as ready for review August 7, 2021 13:43
@RaisinTen RaisinTen force-pushed the test/replace-raw-errno-with-error-code branch from eb85c63 to 046c8ab Compare August 7, 2021 13:43
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@RaisinTen
Copy link
Member Author

RaisinTen commented Aug 7, 2021

How do I run an IBMi CI on this PR just like the one linked in this comment?

CI: https://ci.nodejs.org/job/node-test-commit-ibmi/329/
Test fs_read_type passed - https://ci.nodejs.org/job/node-test-commit-ibmi/329/nodes=ibmi72-ppc64/testReport/(root)/test/parallel_test_fs_read_type/

Originally posted by @dmabupt in #38159 (comment)

@nodejs-github-bot

This comment has been minimized.

Sign in to view
@RaisinTen RaisinTen removed the blocked PRs that are blocked by other issues or PRs. label Aug 7, 2021
@RaisinTen
Copy link
Member Author

RaisinTen commented Aug 14, 2021

cc @nodejs/platform-ibmi some help plz ^

@richardlau
Copy link
Member

richardlau commented Aug 14, 2021

How do I run an IBMi CI on this PR just like the one linked in this comment?

CI: https://ci.nodejs.org/job/node-test-commit-ibmi/329/

https://ci.nodejs.org/job/node-test-commit-ibmi/ is like any other node-test-commit-* job (except for the moment it is not run on every PR because it takes much longer to run on our current machines) -- follow the guide in https://github.com/nodejs/node/blob/master/doc/guides/collaborator-guide.md#starting-a-jenkins-ci-job

@RaisinTen
Copy link
Member Author

RaisinTen commented Aug 15, 2021 •
edited
Loading

IBM i CI: https://ci.nodejs.org/job/node-test-commit-ibmi/493/

@RaisinTen RaisinTen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 15, 2021
@RaisinTen RaisinTen force-pushed the test/replace-raw-errno-with-error-code branch from 046c8ab to 7c8156d Compare August 22, 2021 12:58
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Aug 22, 2021 •
edited by jasnell
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/39633/ 💚

@jasnell
Copy link
Member

jasnell commented Aug 26, 2021

@RaisinTen ... I notice your comment about this being blocked before. Is it still blocked at all?

@RaisinTen
Copy link
Member Author

RaisinTen commented Aug 27, 2021 •
edited
Loading

Hey @jasnell, this is not blocked anymore. I was just trying to get a green IBM i CI on this PR as the changed code path can only be hit on an IBM i. I've been updating #38675 (comment) every time I started a new rebuild but unfortunately, I'm consistently getting a number of ECONNRESETs. Is it worth it to wait for a green CI or should we just land this?

@richardlau
Copy link
Member

richardlau commented Aug 27, 2021

I'm consistently getting a number of ECONNRESETs.

Those started appearing after the recent libuv update and are being tracked in #39683. I did make @nodejs/platform-ibmi aware of them.

@jasnell
Copy link
Member

jasnell commented Aug 28, 2021

Let's go ahead and land this.

jasnell pushed a commit that referenced this pull request Aug 28, 2021
@RaisinTen @jasnell
test: use error code mapping in place of raw errno
21cf618 
Refs: #38159 (comment)

PR-URL: #38675
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@jasnell
Copy link
Member

jasnell commented Aug 28, 2021

Landed in 21cf618

@jasnell jasnell closed this Aug 28, 2021
@RaisinTen RaisinTen deleted the test/replace-raw-errno-with-error-code branch August 29, 2021 01:03
targos pushed a commit that referenced this pull request Sep 6, 2021
@RaisinTen @targos
test: use error code mapping in place of raw errno
bd80966 
Refs: #38159 (comment)

PR-URL: #38675
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
targos pushed a commit that referenced this pull request Sep 6, 2021
@RaisinTen @targos
test: use error code mapping in place of raw errno
007e285 
Refs: #38159 (comment)

PR-URL: #38675
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@targos targos mentioned this pull request Sep 6, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@addaleax addaleax addaleax approved these changes

@cjihrig cjihrig cjihrig approved these changes

@richardlau richardlau richardlau approved these changes

@trivikr trivikr trivikr approved these changes

+1 more reviewer

@jatin1990gupta jatin1990gupta jatin1990gupta approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@RaisinTen @waelsy123 @nodejs-github-bot @richardlau @jasnell @addaleax @cjihrig @trivikr @jatin1990gupta